Skip to content

Agentling/feature/tools#2

Merged
folathecoder merged 6 commits into
mainfrom
agentling/feature/tools
Jul 6, 2026
Merged

Agentling/feature/tools#2
folathecoder merged 6 commits into
mainfrom
agentling/feature/tools

Conversation

@folathecoder

Copy link
Copy Markdown
Owner

No description provided.

Turn a plain function into a model-ready tool. @tool builds a JSON Schema
from type hints (Optional, Literal -> enum, list[str] -> array+items) and a
Google-style docstring; Tool.call validates arguments against that schema
before running, raising ToolCallError on bad input.

- Tool base class (name/description/parameters, async forward, to_schema, call)
- @tool decorator wrapping sync or async functions
- _build_schema (hints -> JSON Schema) + _parse_docstring (per-arg descriptions)
- schema hardening: additionalProperties:false, default exposure, and a
  param.kind guard rejecting *args/**kwargs/positional-only tools
- arg validation: required/unknown/type/enum + non-dict guard -> ToolCallError
- FinalAnswerTool for explicit termination
- JsonSchema vs ToolSpec type split (parameters object vs full function spec)
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add JSON-Schema-backed tools framework, tests, and CI pipeline

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Introduce a @tool decorator and Tool base class with JSON Schema generation.
• Validate model-generated tool arguments and surface errors via ToolCallError.
• Add CI (ruff/mypy/pytest) plus pytest config and unit coverage for tools/models.
Diagram

graph TD
  A["Agent loop"] --> M["OpenAIModel"] --> O{{"OpenAI API"}}
  A --> T["Tool (@tool)"] --> C["Tool.call"] --> F["Tool.forward"]
  T --> S["ToolSpec schema"] --> M
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use jsonschema for validation
  • ➕ Full JSON Schema support (nested objects, oneOf/anyOf, formats, etc.)
  • ➕ Battle-tested error reporting and edge-case handling
  • ➕ Less custom validation logic to maintain
  • ➖ Adds a dependency and potential performance overhead
  • ➖ May encourage exposing more complex schemas than models reliably follow
  • ➖ Error messages might need wrapping to be model-friendly
2. Use Pydantic models for tool arguments
  • ➕ Strong typing + rich validation/coercion with good error messages
  • ➕ Easy default handling and nested structures
  • ➕ Schema generation is built-in
  • ➖ Heavier dependency and runtime cost
  • ➖ Coercion may hide model mistakes you may prefer to surface explicitly
  • ➖ Requires authors to define models, not just type hints/docstrings

Recommendation: The PR’s lightweight subset validation is a good fit for early-stage tooling: it keeps dependencies minimal, rejects unsupported signatures up-front, and provides model-recoverable ToolCallError messages. If/when tool schemas need richer structures (nested objects, unions beyond Optional/Literal), consider switching validation to jsonschema (or Pydantic for stronger coercion) while preserving the current error-surfacing behavior.

Files changed (6) +920 / -2

Enhancement (1) +384 / -0
tools.pyIntroduce Tool abstraction, @tool decorator, schema generation, and validation +384/-0

Introduce Tool abstraction, @tool decorator, schema generation, and validation

• Adds a Tool base class that emits OpenAI-style function ToolSpec schemas and validates arguments before executing. Implements @tool to wrap sync/async functions, generates JSON Schema from type hints + Google-style docstrings, rejects unsupported signatures, and provides FinalAnswerTool for explicit run termination.

src/agentling/tools.py

Tests (2) +463 / -0
test_models.pyAdd unit tests for OpenAIModel conversions, streaming, and retry behavior +290/-0

Add unit tests for OpenAIModel conversions, streaming, and retry behavior

• Introduces tests for message/tool-call conversion, argument parsing, delta agglomeration, and usage capture. Uses a fake OpenAI client to validate generate/stream behavior and rate-limit retry handling without network calls.

tests/test_models.py

test_tools.pyAdd unit tests for tool schema generation and argument validation +173/-0

Add unit tests for tool schema generation and argument validation

• Covers schema generation for primitives, Optional/Union-with-None, Literal enums, list/dict shapes, defaults, and additionalProperties hardening. Verifies sync/async execution via Tool.call, validation failures raising ToolCallError, and FinalAnswerTool behavior.

tests/test_tools.py

Documentation (1) +16 / -2
models.pyImprove models module documentation and method docstrings +16/-2

Improve models module documentation and method docstrings

• Adds a module-level docstring describing the provider-neutral model layer and adapter. Fills in docstrings for Usage.total_tokens and the Model protocol’s generate/stream methods.

src/agentling/models.py

Other (2) +57 / -0
ci.ymlAdd CI workflow for lint, type-check, and tests on 3.11/3.12 +47/-0

Add CI workflow for lint, type-check, and tests on 3.11/3.12

• Introduces a GitHub Actions workflow that runs on PRs and main pushes. Uses uv to install dependencies, then runs ruff, mypy, and pytest across a Python version matrix.

.github/workflows/ci.yml

pyproject.tomlAdd pytest config and dev dependency group +10/-0

Add pytest config and dev dependency group

• Configures pytest (asyncio_mode=auto, tests/ as testpaths). Adds dev dependency group entries for pytest and pytest-asyncio.

pyproject.toml

@folathecoder folathecoder merged commit 925d107 into main Jul 6, 2026
2 checks passed
@folathecoder folathecoder deleted the agentling/feature/tools branch July 6, 2026 12:06
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Optional default schema mismatch 🐞 Bug ≡ Correctness
Description
In agentling.tools, Optional[T]/Union[T, None] is converted to the schema for T, but
_build_schema still emits default = None, yielding an internally inconsistent JSON Schema (e.g.,
type: integer with default: null) that can be rejected by tool-schema validators/providers.
Code

src/agentling/tools.py[R177-303]

+    # Treat Optional[X] as "the field may be omitted". If the field is supplied,
+    # it must still match X. Null is intentionally not advertised as valid.
+    if origin is Union or origin is types.UnionType:
+        non_none = [arg for arg in args if arg is not type(None)]
+        if len(non_none) == 1:
+            return _schema_for_type(non_none[0])
+
+        return {"type": "string"}
+
+    # list[str] becomes an array with typed items. Bare list becomes an array
+    # without item constraints.
+    if origin is list:
+        if args:
+            return {
+                "type": "array",
+                "items": _schema_for_type(args[0]),
+            }
+
+        return {"type": "array"}
+
+    # dict[...] is represented as an object without per-key constraints.
+    if origin is dict:
+        return {"type": "object"}
+
+    # For other generic aliases, fall back to the origin's base type.
+    if origin is not None:
+        return {"type": _JSON_TYPES.get(origin, "string")}
+
+    # Plain types: str, int, float, bool, list, dict, etc.
+    return {"type": _JSON_TYPES.get(python_type, "string")}
+
+
+_ARGS_HEADERS = {
+    "args",
+    "arguments",
+    "parameters",
+    "keyword args",
+    "keyword arguments",
+}
+
+_HEADER_RE = re.compile(r"^([A-Za-z][A-Za-z ]*):$")
+_ARG_RE = re.compile(r"^(\w+)\s*(?:\([^)]*\))?:\s*(.*)$")
+
+
+def _parse_docstring(func: Callable[..., Any]) -> tuple[str, dict[str, str]]:
+    """Extract a summary and argument descriptions from a docstring.
+
+    This is a pragmatic Google-style parser, not a complete docstring parser.
+    It recognizes Args/Arguments/Parameters sections and stops parsing argument
+    descriptions when another section header is encountered.
+    """
+
+    doc = inspect.getdoc(func) or ""
+    summary_lines: list[str] = []
+    arg_docs: dict[str, str] = {}
+
+    section: str | None = None
+    current_arg: str | None = None
+
+    for raw in doc.splitlines():
+        stripped = raw.strip()
+
+        header = _HEADER_RE.match(stripped)
+        if header:
+            key = header.group(1).strip().lower()
+            section = "args" if key in _ARGS_HEADERS else "other"
+            current_arg = None
+            continue
+
+        if section is None:
+            summary_lines.append(stripped)
+
+        elif section == "args":
+            match = _ARG_RE.match(stripped)
+
+            if match:
+                current_arg = match.group(1)
+                arg_docs[current_arg] = match.group(2).strip()
+
+            elif current_arg and stripped:
+                arg_docs[current_arg] += " " + stripped
+
+        # Other sections, such as Returns or Raises, are ignored.
+
+    summary = " ".join(line for line in summary_lines if line).strip()
+    return summary, arg_docs
+
+
+def _build_schema(func: Callable[..., Any]) -> JsonSchema:
+    """Build a JSON Schema parameters object from a function signature."""
+
+    hints = get_type_hints(func)
+    signature = inspect.signature(func)
+    _, arg_docs = _parse_docstring(func)
+
+    properties: dict[str, Any] = {}
+    required: list[str] = []
+
+    for name, param in signature.parameters.items():
+        if name in {"self", "cls"}:
+            continue
+
+        # Tools are invoked with a JSON object of named arguments. Variadic and
+        # positional-only parameters cannot be represented safely, so reject
+        # them during tool registration rather than failing during a model run.
+        if param.kind in {
+            inspect.Parameter.VAR_POSITIONAL,
+            inspect.Parameter.VAR_KEYWORD,
+            inspect.Parameter.POSITIONAL_ONLY,
+        }:
+            raise TypeError(
+                f"Tool {func.__name__!r} has unsupported parameter {name!r}. "
+                "Tools support only normal or keyword-only parameters "
+                "(no *args, **kwargs, or positional-only parameters)."
+            )
+
+        schema = _schema_for_type(hints.get(name, str))
+
+        if name in arg_docs:
+            schema["description"] = arg_docs[name]
+
+        if param.default is inspect.Parameter.empty:
+            required.append(name)
+        else:
+            schema["default"] = param.default
+
+        properties[name] = schema
Evidence
_schema_for_type() explicitly drops None from Union and returns the non-None schema, while
_build_schema() unconditionally attaches schema["default"] = param.default, which will be None
for common Optional parameters, creating a type/default mismatch.

src/agentling/tools.py[177-184]
src/agentling/tools.py[293-303]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_schema_for_type()` unwraps `Optional[T]` into a schema for `T` (int/string/etc), but `_build_schema()` always copies the Python default into `schema["default"]`, including `None`. This produces a JSON Schema where the declared `type` conflicts with the `default` value (`null`), which can cause provider-side schema validation failures when sending tool specs.

### Issue Context
This happens for any tool parameter annotated as `Optional[...]` (or `T | None`) with a default of `None`.

### Fix Focus Areas
- src/agentling/tools.py[177-184]
- src/agentling/tools.py[293-303]

### Implementation notes
Prefer one of:
1) If `param.default is None`, **omit** the `default` key from the JSON Schema property (keeps schema consistent and still allows omission at call-time because Python will use its default).
2) Alternatively, preserve nullability in schema (e.g., `type: [<type>, "null"]` or `anyOf`) and adjust `_matches_json_type()` / validation accordingly to accept `None`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/agentling/tools.py
Comment on lines +177 to +303
# Treat Optional[X] as "the field may be omitted". If the field is supplied,
# it must still match X. Null is intentionally not advertised as valid.
if origin is Union or origin is types.UnionType:
non_none = [arg for arg in args if arg is not type(None)]
if len(non_none) == 1:
return _schema_for_type(non_none[0])

return {"type": "string"}

# list[str] becomes an array with typed items. Bare list becomes an array
# without item constraints.
if origin is list:
if args:
return {
"type": "array",
"items": _schema_for_type(args[0]),
}

return {"type": "array"}

# dict[...] is represented as an object without per-key constraints.
if origin is dict:
return {"type": "object"}

# For other generic aliases, fall back to the origin's base type.
if origin is not None:
return {"type": _JSON_TYPES.get(origin, "string")}

# Plain types: str, int, float, bool, list, dict, etc.
return {"type": _JSON_TYPES.get(python_type, "string")}


_ARGS_HEADERS = {
"args",
"arguments",
"parameters",
"keyword args",
"keyword arguments",
}

_HEADER_RE = re.compile(r"^([A-Za-z][A-Za-z ]*):$")
_ARG_RE = re.compile(r"^(\w+)\s*(?:\([^)]*\))?:\s*(.*)$")


def _parse_docstring(func: Callable[..., Any]) -> tuple[str, dict[str, str]]:
"""Extract a summary and argument descriptions from a docstring.

This is a pragmatic Google-style parser, not a complete docstring parser.
It recognizes Args/Arguments/Parameters sections and stops parsing argument
descriptions when another section header is encountered.
"""

doc = inspect.getdoc(func) or ""
summary_lines: list[str] = []
arg_docs: dict[str, str] = {}

section: str | None = None
current_arg: str | None = None

for raw in doc.splitlines():
stripped = raw.strip()

header = _HEADER_RE.match(stripped)
if header:
key = header.group(1).strip().lower()
section = "args" if key in _ARGS_HEADERS else "other"
current_arg = None
continue

if section is None:
summary_lines.append(stripped)

elif section == "args":
match = _ARG_RE.match(stripped)

if match:
current_arg = match.group(1)
arg_docs[current_arg] = match.group(2).strip()

elif current_arg and stripped:
arg_docs[current_arg] += " " + stripped

# Other sections, such as Returns or Raises, are ignored.

summary = " ".join(line for line in summary_lines if line).strip()
return summary, arg_docs


def _build_schema(func: Callable[..., Any]) -> JsonSchema:
"""Build a JSON Schema parameters object from a function signature."""

hints = get_type_hints(func)
signature = inspect.signature(func)
_, arg_docs = _parse_docstring(func)

properties: dict[str, Any] = {}
required: list[str] = []

for name, param in signature.parameters.items():
if name in {"self", "cls"}:
continue

# Tools are invoked with a JSON object of named arguments. Variadic and
# positional-only parameters cannot be represented safely, so reject
# them during tool registration rather than failing during a model run.
if param.kind in {
inspect.Parameter.VAR_POSITIONAL,
inspect.Parameter.VAR_KEYWORD,
inspect.Parameter.POSITIONAL_ONLY,
}:
raise TypeError(
f"Tool {func.__name__!r} has unsupported parameter {name!r}. "
"Tools support only normal or keyword-only parameters "
"(no *args, **kwargs, or positional-only parameters)."
)

schema = _schema_for_type(hints.get(name, str))

if name in arg_docs:
schema["description"] = arg_docs[name]

if param.default is inspect.Parameter.empty:
required.append(name)
else:
schema["default"] = param.default

properties[name] = schema

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Optional default schema mismatch 🐞 Bug ≡ Correctness

In agentling.tools, Optional[T]/Union[T, None] is converted to the schema for T, but
_build_schema still emits default = None, yielding an internally inconsistent JSON Schema (e.g.,
type: integer with default: null) that can be rejected by tool-schema validators/providers.
Agent Prompt
### Issue description
`_schema_for_type()` unwraps `Optional[T]` into a schema for `T` (int/string/etc), but `_build_schema()` always copies the Python default into `schema["default"]`, including `None`. This produces a JSON Schema where the declared `type` conflicts with the `default` value (`null`), which can cause provider-side schema validation failures when sending tool specs.

### Issue Context
This happens for any tool parameter annotated as `Optional[...]` (or `T | None`) with a default of `None`.

### Fix Focus Areas
- src/agentling/tools.py[177-184]
- src/agentling/tools.py[293-303]

### Implementation notes
Prefer one of:
1) If `param.default is None`, **omit** the `default` key from the JSON Schema property (keeps schema consistent and still allows omission at call-time because Python will use its default).
2) Alternatively, preserve nullability in schema (e.g., `type: [<type>, "null"]` or `anyOf`) and adjust `_matches_json_type()` / validation accordingly to accept `None`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant